return bool instead of np.bool_
#4074
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Beautiful! I believe I have seen issues related to this at some point. I just did a global search (re pattern pymatgen/src/pymatgen/analysis/chemenv/utils/coordination_geometry_utils.py Lines 716 to 727 in ea7c339 pymatgen/src/pymatgen/core/surface.py Lines 308 to 321 in ea7c339 The following might return pymatgen/src/pymatgen/analysis/interfaces/zsl.py Lines 317 to 330 in ea7c339 |
|
I did some messy The Slightly updated list: |
|
I think this script can be adapted later to handle a more thorough check of serialization bugs caused by custom import pytest
import inspect
import pkgutil
import importlib
import pymatgen
import numpy as np
triggered = set()
def log_numpy_return(func, path_name, name):
def wrapper(*args_, **kwargs_):
result = func(*args_, **kwargs_)
if isinstance(result, np.generic):
# print(f"Function '{name}' in '{path_name}' returned a NumPy generic object")
triggered.add(f"Function '{name}' in '{path_name}' returned a NumPy generic object")
return result
return wrapper
def wrap_func(path, name, func, path_name):
setattr(path, name, log_numpy_return(func, path_name, name))
def get_all_functions(package):
"""
Get all functions and methods from a package and its submodules.
"""
functions = dict()
for module_info in pkgutil.walk_packages(package.__path__, prefix=f"{package.__name__}."):
try:
# Dynamically import the module
module = importlib.import_module(module_info.name)
# Get all functions in the module
for name, func_ in inspect.getmembers(module, inspect.isfunction):
source_module = func_.__module__
if package.__name__ not in source_module: continue
functions[f"{module.__name__}.{name}"] = (module, name, func_, source_module)
# Get all the methods in the module
for class_name, obj in inspect.getmembers(module, inspect.isclass):
source_module = obj.__module__
if package.__name__ not in source_module: continue
for name2, method_ in inspect.getmembers(obj, inspect.isfunction):
functions[f"{module.__name__}.{class_name}.{name2}"] = (obj, name2, method_, f"{source_module}.{class_name}")
except Exception as e:
# Handle modules that cannot be imported or have issues
print(f"Failed to inspect {module_info.name}: {e}")
return functions
funcs = get_all_functions(pymatgen)
for v in funcs.values():
wrap_func(*v)
# Run pytest
pytest.main(['-v', '-s'])
print("Triggered:")
print(triggered)
# write triggerd to file
with open("triggered.txt", "w") as f:
f.write("\n".join(triggered)) |
|
Ok I went through the list and found ones where a user will plausibly serialize into a DB. |
|
Wow, that's way more thorough than mine :)
I thought import numpy as np
def foo() -> bool:
return np.bool_(True) >>> Incompatible return value type (got "numpy.bool", expected "builtins.bool") [return-value]I'm not seeing Lines 282 to 287 in ea7c339 |
|
Yeah |
|
Will go ahead and merge this, thanks both |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
The return signature of this function is currently
np.bool_which can cause serialization problems with older atomate1 workflows.Changed to
boolinstead.